-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Короче, пусть дальше @hrrrrustic cмотрит
Source/SeaInk.Application/TableLayout/Components/AssignmentColumnComponent.cs
Show resolved
Hide resolved
А я тут причем вообще... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я ничего не понял. ООП порнухи больше чем смысловой нагрузки кода
Source/SeaInk.Application/Exceptions/MissingAssignmentComponentException.cs
Show resolved
Hide resolved
Source/SeaInk.Application/TableLayout/CommandInterfaces/IValueGettingLayoutComponent.cs
Outdated
Show resolved
Hide resolved
|
||
public AggregateValuesCommand(ITableDataProvider provider) | ||
{ | ||
_provider = provider.ThrowIfNull(nameof(provider)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
копипастни с 6 нета этот метод, чтобы не делать неймоф
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Жёстко надристал полный пр TryExecute, может посмотреть в сторону FluentResult?
Source/SeaInk.Application/TableLayout/Components/PlainAssignmentColumnComponent.cs
Outdated
Show resolved
Hide resolved
|
||
namespace SeaInk.Application.Exceptions | ||
{ | ||
[Serializable] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не резолвь этот комент пока не поменяешь кофиг сонара
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я так понимаю, что здесь должен быть пункт Administration
чтобы я смог это сделать @FrediKats
public interface IReducibleLayoutComponent<in TComponent> | ||
where TComponent : LayoutComponent | ||
{ | ||
bool TryRemoveComponent(TComponent component, IScaledTableIndex begin, ITableEditor editor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не очень понял как Reducible связано с Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expandable (расширяемый) -> Add component
Reducible (уменьшаемый) -> Remove component
Если такой нейминг кажется плохим, хоть он и корректен в плане инглиша, могу предложить поменять имя интерфейса на IShrinkableLayoutComponent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Думаю что Reducible и Shrinkable эквивалентны, так что особой разницы нет (хотя второе мне импонирует больше)
Может стоит подумать и как-то обозначить то, что убираем именно child компонент
Source/SeaInk.Application/TableLayout/Commands/AggregateValuesCommand.cs
Outdated
Show resolved
Hide resolved
Source/SeaInk.Application/TableLayout/Commands/AggregateValuesCommand.cs
Outdated
Show resolved
Hide resolved
Source/SeaInk.Application/TableLayout/Commands/ComponentRemoveCommand.cs
Outdated
Show resolved
Hide resolved
Source/SeaInk.Application/TableLayout/Commands/DrawAllCommand.cs
Outdated
Show resolved
Hide resolved
…ntColumnComponent.cs Co-authored-by: Bibletoon <45363221+Bibletoon@users.noreply.github.com>
refactor: fallthrough commands
оаоаооа ммм натягивание фарш штук на сишарп |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поправь только перед тем как мерджить
Kudos, SonarCloud Quality Gate passed! |
Closes #9